chore: use ESM exports where appropriate #151
Conversation
| @@ -191,7 +191,7 @@ async function setupAndRun() { | |||
| } | |||
| } | |||
|
|
|||
| module.exports = { | |||
| export default { | |||
There was a problem hiding this comment.
I wonder whether this shouldn't be export run and export init
| @@ -34,4 +34,4 @@ const withPods = { | |||
| ios: ios.pod, | |||
| }; | |||
|
|
|||
| module.exports = { flat, nested, withExamples, withPods }; | |||
| export default { flat, nested, withExamples, withPods }; | |||
There was a problem hiding this comment.
Shouldn't this be a series of named exports instead of an object?
| @@ -43,7 +43,7 @@ function getDevices(): Array<string> { | |||
| } | |||
| } | |||
|
|
|||
| module.exports = { | |||
| export default { | |||
| parseDevicesResult, | |||
There was a problem hiding this comment.
Shouldn't this be a set of named exports?
| @@ -29,7 +29,7 @@ const debug = (...messages: Array<string>) => { | |||
| console.log(`${chalk.black.bgWhite(' DEBUG ')} ${joinMessages(messages)}`); | |||
| }; | |||
|
|
|||
| module.exports = { | |||
| export default { | |||
| @@ -52,7 +52,7 @@ function isGlobalCliUsingYarn(projectDir) { | |||
| return fs.existsSync(path.join(projectDir, 'yarn.lock')); | |||
| } | |||
|
|
|||
| module.exports = { | |||
| export default { | |||
| @@ -71,7 +71,7 @@ function remove(packageName, projectDir) { | |||
| ); | |||
| } | |||
|
|
|||
| module.exports = { | |||
| export default { | |||
There was a problem hiding this comment.
Final round, it looks good to me. I just wanted to suggest we refine some export default's when there's an object exported.
It makes sense to have export default {} in case of a command where properties of an object are not meant to be consumed directly.
CC: @thymikee
|
@grabbou Ok, now I got it. Will add. |
|
Awesome @sidferreira! Thank you |
packages/cli/src/index.js
Outdated
| @@ -17,4 +17,4 @@ if (require.main === module) { | |||
| cli.run(); | |||
| } | |||
|
|
|||
| module.exports = cli; | |||
| export default cli; | |||
There was a problem hiding this comment.
please revert that, this is public API
|
@grabbou the changes you requested ended being larger than expected. I reverted until we decide if we do the extra work now or wait for later. |
|
@sidferreira sounds good. Did you update the public API? |
|
@thymikee please update your review so we can land it! |
|
Thanks @sidferreira 👍 |
This is an extra for #150
Fixes exports and tweaks some tests.
The
link,linkAssets, andlinkDependencywhere trickier as hell, but we have all tests working.I suggest take it easy in this review as I'm worried in a "false positive" in tests as there was a LOT of changes.
We need to improve the code coverage for changes like this have smaller chances of failing...